Fix sanitizeHTML performance overhead and HTML-in-translation strings#7
Open
datengraben wants to merge 1 commit into
Open
Fix sanitizeHTML performance overhead and HTML-in-translation strings#7datengraben wants to merge 1 commit into
datengraben wants to merge 1 commit into
Conversation
… (issue wielebenwir#2043) Addresses three problems reported by @sbomsdorf: 1. Performance: commonsbooking_sanitizeHTML() rebuilt its $allowed_atts array and mutated the global $allowedposttags on every single call. On an admin settings page this runs ~200 times. Replace both with a static variable that merges the tags list once per request and passes it directly to wp_kses(), leaving the global untouched. 2. Translation complexity (OptionsArray.php, CB1UserFields.php): 111 'title', 'name', 'default', and 'group_title' array entries wrapped plain text strings in commonsbooking_sanitizeHTML() at definition time. Plain text requires no sanitization; the wrapper added CPU cost and confused static analysis. Removed the wrapper — escaping at render time is handled by CMB2 and the calling template (late escape pattern from issue wielebenwir#2043). 3. HTML-in-translation strings: - UserWidget: <li><a href="%s">...</a></li> patterns replaced with explicit HTML construction + esc_url()/esc_html__(), so translators only see the link label, not fragile markup. - CB1UserFields::get_termsservices_string(): <a href> inside __() replaced with esc_url() + esc_html__(); also fixes a pre-existing malformed attribute (target=_blank" → target="_blank"). https://claude.ai/code/session_01P1DDnvRKzHdx7jepeTf3ju
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
$(cat <<'EOF'
Summary
Addresses the three issues raised by @sbomsdorf in wielebenwir#2043.
1. Performance —
commonsbooking_sanitizeHTML()rebuilt on every callBefore: every call rebuilt a 29-key
$allowed_attsarray from scratch and overwrote those entries on the global$allowedposttags. On a single admin settings page load this runs ~200 times — pure redundant CPU work.After: a
static $allowed_tags = nullvariable builds the merged allowed-tags list once per request viaarray_merge()and passes it directly towp_kses(). The global$allowedposttagsis no longer mutated.2. Late escape — plain-text
title/name/defaultfields in settings arrays111 entries across
includes/OptionsArray.phpandsrc/CB/CB1UserFields.phpwrapped plain-text translation strings incommonsbooking_sanitizeHTML()at array-definition time, e.g.:Plain text contains no HTML to sanitize. The wrapper was removed; CMB2 applies its own escaping at render time (late-escape pattern). The remaining
commonsbooking_sanitizeHTML()calls in those files are all legitimately applied to HTML-containingdescand multi-line email-bodydefaultvalues.3. HTML out of translation strings
<li><a href="%s">...</a></li>patterns inside__()replaced with explicit PHP string construction usingesc_url()+esc_html__(). Translators now only see the plain link label, not fragile markup they could accidentally break.Files changed:
src/Wordpress/Widget/UserWidget.php— 5 list-item link stringssrc/CB/CB1UserFields.php— terms-and-services link; also fixes pre-existing malformed attributetarget=_blank"→target="_blank"Test plan
target="_blank"attributecomposer test— PHPUnit suite passeshttps://claude.ai/code/session_01P1DDnvRKzHdx7jepeTf3ju
EOF
)